Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix rpc call from name based Callables #87858

Merged
merged 1 commit into from
Mar 11, 2024
Merged

Fix rpc call from name based Callables #87858

merged 1 commit into from
Mar 11, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Feb 2, 2024 •
edited
Loading

Makes rpc calls work for the Callable(obj, method) form Callables, works like the GDScriptRPCCallable call (which also does no special treatment for the error return, I'll leave it as such and leave any improvements or changes to that side to future improvements to both)

@AThousandShips AThousandShips added bug topic:core cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Feb 2, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Feb 2, 2024
@AThousandShips AThousandShips requested a review from a team as a code owner February 2, 2024 08:29
@SpazElectro SpazElectro mentioned this pull request Feb 7, 2024
Closed
@akien-mga akien-mga requested review from Faless and KoBeWi March 6, 2024 12:50
@AThousandShips AThousandShips mentioned this pull request Mar 10, 2024
Closed
KoBeWi
KoBeWi reviewed Mar 10, 2024
View reviewed changes
core/variant/callable.cpp Outdated Show resolved Hide resolved
KoBeWi
KoBeWi approved these changes Mar 10, 2024
View reviewed changes
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the error, but not sure how to confirm whether the method is actually called.
The code looks fine though.

Faless
Faless approved these changes Mar 11, 2024
View reviewed changes
Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me!

I remember @reduz wanted this to be implemented at the language level using custom callables, but I think it make sense to also have the option of using Callable(obj, method) as an alternative if the language doesn't implement it.

@akien-mga akien-mga merged commit 22dcfa1 into godotengine:master Mar 11, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the callable_rpc branch March 11, 2024 13:41
@AThousandShips
Copy link
Member Author

Thank you!

@raulsntos raulsntos mentioned this pull request Jul 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga left review comments

@KoBeWi KoBeWi KoBeWi approved these changes

@Faless Faless Faless approved these changes

Assignees
No one assigned
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:core
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

Callable.rpc() on RPC function throws an error "Invalid call. Nonexistent function 'rpc' in base 'Callable'.
4 participants
@AThousandShips @akien-mga @Faless @KoBeWi